Expose `Stripe::Webhook.compute_signature` #915

brandur · 2020-04-24T17:22:50Z

Exposes the .compute_signature method, which may be useful when
testing webhook signing in test suites.

I change the API slightly so that a caller isn't forced to do as much
string mangling, and to match the one that we already have in stripe-go:

func ComputeSignature(t time.Time, payload []byte, secret string) []byte {

Add basic documentation and test case. I also change a few things around
so that we send Time objects around more often where applicable, and
don't change then to Unix integers until the last moment that we need
to.

The one other alternative API I considered is this one, which would
default the timestamp to the current time to allow the method to be
called with one fewer arg:

def self.compute_signature(payload, secret, timestamp: Time.now)

I decided against it in the end though because it does remove some
explicitness, and it's not a big deal to just pass in Time.now,
especially given that this is not expected to be a commonly used method.

Fixes #912.

r? @ob-stripe
cc @stripe/api-libraries

Exposes the `.compute_signature` method, which may be useful when testing webhook signing in test suites. I change the API slightly so that a caller isn't forced to do as much string mangling, and to match the one that we already have in stripe-go: ``` go func ComputeSignature(t time.Time, payload []byte, secret string) []byte { ``` Add basic documentation and test case. I also change a few things around so that we send `Time` objects around more often where applicable, and don't change then to Unix integers until the last moment that we need to. The one other alternative API I considered is this one, which would default the timestamp to the current time to allow the method to be called with one fewer arg: ``` ruby def self.compute_signature(payload, secret: timestamp: Time.now) ``` I decided against it in the end though because it does remove some explicitness, and it's not a big deal to just pass in `Time.now`, especially given that this is not expected to be a commonly used method. Fixes #912.

brandur-stripe · 2020-04-24T17:58:38Z

Thanks as usual OB.

sandstrom · 2020-04-24T21:05:46Z

@brandur-stripe Awesome, much appreciated!

jarthod · 2020-04-25T07:49:15Z

Nice, that simplifies a bit the testing code:

Out of curiosity though is there a way to avoid hardcoding the whole header construct?, something like:

timestamp = Time.now
signature = Stripe::Webhook::Signature.compute_signature(timestamp, event_json, secret)
scheme = Stripe::Webhook::Signature::EXPECTED_SCHEME
"t=#{timestamp.to_i},#{scheme}=#{signature}"

⬇️

Stripe::Webhook::Signature.compute_signature_header(Time.now, event_json, secret)

That could be nice ;)

brandur-stripe · 2020-04-27T18:45:57Z

Ah interesting — yeah, something like that makes sense.

@ob-stripe Can I get your opinion on this one? The method proposed above would look like this:

      # Computes the value that would be added to a `Stripe-Signature` for a
      # given webhook payload.
      #
      # Note that this isn't needed to verify webhooks in any way, and is
      # mainly here to make writing test cases easier.
      def self.compute_signature_header(timestamp, payload, secret)
        raise ArgumentError, "timestamp should be an instance of Time" \
          unless timestamp.is_a?(Time)
        raise ArgumentError, "payload should be a string" \
          unless payload.is_a?(String)
        raise ArgumentError, "secret should be a string" \
          unless secret.is_a?(String)

        signature = compute_signature(timestamp, payload, secret)
        "t=#{timestamp.to_i},#{EXPECTED_SCHEME}=#{signature}"
      end

But I was thinking that we might want something a little more granular, where you have to generate a signature separately and inject it:

      # Generates a value that would be added to a `Stripe-Signature` for a
      # given webhook payload.
      #
      # Note that this isn't needed to verify webhooks in any way, and is
      # mainly here to make writing test cases easier.
      def self.generate_header(timestamp, signature, scheme: Stripe::Webhook::Signature::EXPECTED_SCHEME)
        raise ArgumentError, "timestamp should be an instance of Time" \
          unless timestamp.is_a?(Time)
        raise ArgumentError, "signature should be a string" \
          unless signature.is_a?(String)
        raise ArgumentError, "scheme should be a string" \
          unless scheme.is_a?(String)

        "t=#{timestamp.to_i},#{scheme}=#{signature}"
      end

Calling this is a little more work, but more flexible (like in case you already have a signature):

t = Time.now
payload = ...
signature = Stripe::Webhook::Signature.compute_signature(t, payload, SECRET)
header = Stripe::Webhook::Signature.generate_header(t, signature)

Thoughts?

brandur-stripe · 2020-04-27T18:46:44Z

Oh, and I forgot to mention: the other downside is that this would be a method in the API that's really only relevant for tests. Not sure how you feel about that in general.

ob-stripe · 2020-04-27T19:58:28Z

I can definitely see how it would be useful, even if it's just meant to be used in tests. We kind of have something like that in our own test suite:

stripe-ruby/test/stripe/webhook_test.rb

Lines 15 to 26 in 325ff57

    
           def generate_header(opts = {}) 
        
             opts[:timestamp] ||= Time.now 
        
             opts[:payload] ||= EVENT_PAYLOAD 
        
             opts[:secret] ||= SECRET 
        
             opts[:scheme] ||= Stripe::Webhook::Signature::EXPECTED_SCHEME 
        
             opts[:signature] ||= Stripe::Webhook::Signature.compute_signature( 
        
               opts[:timestamp], 
        
               opts[:payload], 
        
               opts[:secret] 
        
             ) 
        
             "t=#{opts[:timestamp].to_i},#{opts[:scheme]}=#{opts[:signature]}" 
        
           end

If we add this, I'd prefer the more granular version as well.

Adds a new `generate_header` method for the webhooks module, following up #915. This method doesn't help in any way with webhook verification, but may be useful to users in their test suites as it allows them to easily simulate the contents of a header that Stripe might have sent. We briefly discussed an alternative design here, but this one seems like the best fit: #915 (comment)

brandur-stripe · 2020-04-27T21:31:26Z

Thanks OB. Opened #916.

Adds a new `generate_header` method for the webhooks module, following up #915. This method doesn't help in any way with webhook verification, but may be useful to users in their test suites as it allows them to easily simulate the contents of a header that Stripe might have sent. We briefly discussed an alternative design here, but this one seems like the best fit: #915 (comment)

As of 5.19.0, the compute_signature method was made public and they split the timestamp into a separate param (which must be a Time). This version checks compute_signature based on the number of params it takes since doing a version check felt odd. Now that it is public, the method shouldn't change it's API. 1: stripe/stripe-ruby#915

stripe-ci assigned ob-stripe Apr 24, 2020

ob-stripe approved these changes Apr 24, 2020

View reviewed changes

stripe-ci added the approved label Apr 24, 2020

brandur-stripe merged commit e117c9f into master Apr 24, 2020

sandstrom mentioned this pull request Apr 24, 2020

Example generate signature #913

Closed

ob-stripe deleted the brandur-expose-compute-signature branch April 27, 2020 19:13

brandur mentioned this pull request Apr 27, 2020

Add new .generate_header method for webhooks #916

Merged

gaffneyc mentioned this pull request May 12, 2020

Fixes tests for Stripe 5.19.0 integrallis/stripe_event#133

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Expose `Stripe::Webhook.compute_signature` #915

Expose `Stripe::Webhook.compute_signature` #915

brandur commented Apr 24, 2020 •

edited by brandur-stripe

brandur-stripe commented Apr 24, 2020

sandstrom commented Apr 24, 2020

jarthod commented Apr 25, 2020

brandur-stripe commented Apr 27, 2020

brandur-stripe commented Apr 27, 2020

ob-stripe commented Apr 27, 2020

brandur-stripe commented Apr 27, 2020

Expose Stripe::Webhook.compute_signature #915

Expose Stripe::Webhook.compute_signature #915

Conversation

brandur commented Apr 24, 2020 • edited by brandur-stripe

brandur-stripe commented Apr 24, 2020

sandstrom commented Apr 24, 2020

jarthod commented Apr 25, 2020

brandur-stripe commented Apr 27, 2020

brandur-stripe commented Apr 27, 2020

ob-stripe commented Apr 27, 2020

brandur-stripe commented Apr 27, 2020

Expose `Stripe::Webhook.compute_signature` #915

Expose `Stripe::Webhook.compute_signature` #915

brandur commented Apr 24, 2020 •

edited by brandur-stripe